fix(routes): take a route's handler from the last argument, not the first - #2009

Approved, and this is the more consequential of your two route fixes — a wrong edge, not a missing one.

Verified on main: extract_handler_arg (extract_calls.c:2351) returns inside the loop on the first eligible argument, and MAX_HANDLER_SCAN = 4 with HANDLER_START_IDX = 1 means it only ever examines arguments 1–3. So both faults you describe are real:

• routerGet("/users", requireAuth, rateLimit, listUsers) → returns requireAuth, which second_arg_name carries into pass_calls.c as the HANDLES target. The graph then says the route is handled by the auth middleware.
• Three inline arrow middlewares push the real handler to index 4, past the cap, and nothing comes back at all.

A wrong edge is worse than a missing one, and worth saying why: a missing HANDLES edge is visibly absent, while a wrong one is silently plausible — anyone tracing a request through the graph lands on requireAuth, sees a function that really is on that route, and has no signal that it is not the handler. That is the failure mode this project cares most about.

Taking the last eligible argument is the right rule for the frameworks named — Express, Fastify, gin and Laravel all place middleware between path and handler with the handler last — and your justification is the part that makes it safe: nothing that legitimately follows a handler matches the accepted kinds, since an options argument is an object node.

One thing to keep an eye on, not a blocker. is_string_like is among the accepted kinds, because Laravel handlers are strings ('Controller@method'). Under first-match a trailing non-handler string could never win; under last-match it can. I could not construct a real call shape where that bites — a trailing string argument after a handler is not a pattern any of the four frameworks use — but it is the one place where "last" is a weaker assumption than "first", so it is worth remembering if a Laravel route ever reports an odd HANDLES target.

Removing MAX_HANDLER_SCAN entirely rather than raising it is correct: a cap on a scan that must find the last match has no meaning. Keeping HANDLER_START_IDX because argument 0 really is the route path is the right asymmetry.

Both faults reproduced red before the change, with the distinct failure text for each ("requireAuth" != "listUsers" and second_arg_name is NULL), which is what shows they are two faults and not one.

Merging on green. Our Actions queue is heavily backlogged at the moment, so expect a wait — nothing to do with this PR. #2008 touches extract_defs.c and this touches extract_calls.c, so the two are independent and neither will need a rebase for the other.

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Merged as 01bf5f23. Thank you — this was the more consequential of your two route fixes.

Worth restating why: extract_handler_arg returned the first eligible argument, so routerGet("/users", requireAuth, rateLimit, listUsers) produced a HANDLES edge pointing at the auth middleware. That is a wrong edge, not a missing one — and a wrong edge is the worse failure, because anyone tracing a request lands on a function that really is on that route and has no signal it is not the handler. A missing edge is visibly absent; a wrong one is silently plausible.

Removing MAX_HANDLER_SCAN rather than raising it was right too: a cap on a scan that must find the last match has no meaning. And keeping HANDLER_START_IDX because argument 0 really is the route path is the correct asymmetry.

On verification

Your green was 35/35, but main had taken six merges since your branch's base — three of which touched tests/test_extraction.c, including #1824, which is also route-related. Rather than argue that the production files were disjoint, I rebuilt the actual merge locally against current main and ran extraction and edge_types_probe: clean build, 388 passed, 0 failed. That cost our badly-backlogged Actions queue nothing and settled the question properly.

The one thing I noted at review still stands as a watch item, not a blocker: is_string_like is among the accepted kinds for Laravel string handlers, so "last match" is a weaker assumption than "first match" for a trailing string argument. I could not construct a real call shape where it bites, but if a Laravel route ever reports an odd HANDLES target, that is the first place to look.

That is your fourth merge today, after #2008, #1986 and #1877.